STYLE: Format code using `ruff` #881

jhlegarreta · 2024-04-27T16:37:40Z

Format the code using ruff: supersedes flake8 and blue.

blue was not being applied previously as the relevant pre-commit hook config block was commented in the.

Remove the .flake8 config file: in practice few rules were being applied as the config file contained a significant number fo ignored rules.

Bump ruff-pre-commit pre-commit hook version to v0.4.3 (May 3, 2024): the version being used previously was v0.1.7 was from Dec 4, 2023. Made changes consistent between changes being observed locally vs on the GitHub action.

Sort the imports according to the new version: new sorting is dictated by the default value of force-sort-within-sections (false): imports are sorted by module.

jhlegarreta · 2024-04-27T16:53:44Z

The pre-commit hook is nicely doing its job:
https://github.com/fury-gl/fury/actions/runs/8861237757/job/24332897396?pr=881#step:4:23191

codecov · 2024-04-27T17:15:27Z

Codecov Report

Attention: Patch coverage is 72.82828% with 269 lines in your changes are missing coverage. Please review.

Project coverage is 84.37%. Comparing base (cd3d742) to head (65a12d6).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #881      +/-   ##
==========================================
- Coverage   84.43%   84.37%   -0.06%     
==========================================
  Files          44       44              
  Lines       10534    10556      +22     
  Branches     1423     1432       +9     
==========================================
+ Hits         8894     8907      +13     
- Misses       1266     1270       +4     
- Partials      374      379       +5

Files	Coverage Δ
fury/actors/tensor.py	`96.39% <100.00%> (ø)`
fury/animation/__init__.py	`100.00% <100.00%> (ø)`
fury/animation/helpers.py	`92.00% <ø> (ø)`
fury/convert.py	`80.00% <100.00%> (ø)`
fury/data/__init__.py	`100.00% <100.00%> (ø)`
fury/decorators.py	`100.00% <100.00%> (ø)`
fury/molecular.py	`100.00% <100.00%> (ø)`
fury/shaders/__init__.py	`100.00% <ø> (ø)`
fury/stream/constants.py	`100.00% <100.00%> (ø)`
fury/stream/server/__init__.py	`100.00% <100.00%> (ø)`
... and 33 more

skoudoro

Hi @jhlegarreta,

Thank you for this. See my comment below

pyproject.toml

.pep8speaks.yml

.pre-commit-config.yaml

pyproject.toml

skoudoro

Hi @jhlegarreta,

See above my comment. In summary:

ok for double quotes
remove experimental from linting please

jhlegarreta · 2024-04-30T22:18:41Z

@skoudoro OK. Please, have a look at the CI result to see if the changes look OK to you, and if so, I will apply them locally and commit all files.

skoudoro · 2024-04-30T22:27:39Z

Looking into it. I might merge #885 and #883 before you do that.

skoudoro · 2024-04-30T22:31:10Z

I just skim it. A lot is single quotes vs double quotes. Not sure I will have time now to check if there is something weird that I do not agree.

So for now, I agree and if something come up, it will be updated on a new PR.

jhlegarreta · 2024-04-30T22:36:18Z

Looking into it. I might merge #885 and #883 before you do that.

So for now, I agree and if something come up, it will be updated on a new PR.

OK. Will keep an eye on those PRs. If the issues in there are not addressed by Friday, I will lint the files as told by ruff here and will commit them.

skoudoro · 2024-04-30T22:51:24Z

@WassCodeur, please, see the comment above. You need to update all your PR before thursday afternoon

WassCodeur · 2024-05-01T02:46:14Z

sure

jhlegarreta · 2024-05-04T00:49:31Z

Some comments

Add 2 exclusion rules (besides some paths)
Running
```
ruff --config pyproject.toml --fix
```
detected some stuff that I had to undo so that tests pass, e.g.
https://github.com/fury-gl/fury/blob/master/fury/ui/elements.py#L3232
warnings.warn stacklevels and double quotes:
https://github.com/fury-gl/fury/blob/master/fury/tests/test_testing.py#L105
Writing as literals:
https://github.com/fury-gl/fury/blob/master/fury/deprecator.py#L320
https://github.com/fury-gl/fury/blob/master/docs/source/ext/github_tools.py#L412

Probably my fixes were not good.

Not sure why all the changes that the above command proposed were not being reported in the CI in the first place, or why the empty line before the first import is enforced only depending on how we call the commands (above vs ruff format --diff).

Looks like I'm missing something (GHA command outcome vs what I would expect, locally running the commands).
No sure why the exclude rules
https://github.com/fury-gl/fury/pull/881/files#diff-50c86b7ed8ac2cf95bd48334961bf0530cdc77b5a56f852c5c61b89d735fd711R108

are not being applied:
https://github.com/fury-gl/fury/actions/runs/8946250187/job/24576577739#step:4:188

Same goes to the # codespell:ignore in fury/shaders.base.py:
https://github.com/fury-gl/fury/actions/runs/8946250187/job/24576577739#step:4:146

Anyways, I think the above differences can be worked in a separate PR: many of the edits in this PR were not trivial/were manual, and avoiding to go over all of them again would save time, e.g.

So I'd merge this as is and rework the rest in separate PRs. @skoudoro let me know if you can afford investigating the above a bit or whether merging this is better.

skoudoro · 2024-05-04T01:07:19Z

Thanks for this huge work.

Not sure to understand all your request but I will go carefully through this PR. if all ok, I will go ahead and merge, then, we will fix the other issue in new PRs

jhlegarreta · 2024-05-04T02:41:23Z

👍 One of the things that is maybe happening is that the pyproject.toml config options are not being taken into account for the commands above/GitHub format action: when running ruff --config pyproject.toml --fix changes to the pyproject.toml file are taken into account (e.g. the excluded paths). The action and the .pre-commit-config are pretty standard, and I've seen them being used in many places. It does not explain everything, but it does explain some differences.

Edit: I inadvertently closed the PR. Re-opened.

Format the code using `ruff`: supersedes `flake8` and `blue`. `blue` was not being applied previously as the relevant `pre-commit` hook config block was commented in the. Remove the `.flake8` config file: in practice few rules were being applied as the config file contained a significant number fo ignored rules. Bump `ruff-pre-commit` pre-commit hook version to v0.4.3 (May 3, 2024): the version being used previously was v0.1.7 was from Dec 4, 2023. Made changes consistent between changes being observed locally vs on the GitHub action. Sort the imports according to the new version: new sorting is dictated by the default value of `force-sort-within-sections` (`false`): imports are sorted by module. Fix the warnings reported by `ruff`, e.g. C408, F401, etc.

jhlegarreta · 2024-05-05T01:46:17Z

OK, got this working: the issue was that there is a ruff.toml file that I had completely forgotten about, and I was adding ruff rules to pyproject.toml, the rules int ruff.toml taking precedence. Tested that the pre-commit hook is raising all these on my fork:
https://github.com/jhlegarreta/fury/actions/runs/8953728977/job/24592533047?pr=3

And here fixes have been applied.

Sorry (esp @skoudoro) for all the noise generated by the push forces and the previous misleading comments.

This is ready to be merged (IMHO, the Ubuntu Python 3.9 failure is unrelated).

skoudoro

thank you @jhlegarreta,

merging

jhlegarreta force-pushed the TransitionToRuffForFormatting branch 2 times, most recently from 560830b to ea4ccc0 Compare April 27, 2024 16:43

skoudoro requested changes Apr 28, 2024

View reviewed changes

pyproject.toml Outdated Show resolved Hide resolved

.pep8speaks.yml Outdated Show resolved Hide resolved

.pre-commit-config.yaml Show resolved Hide resolved

pyproject.toml Show resolved Hide resolved

pyproject.toml Outdated Show resolved Hide resolved

jhlegarreta requested a review from skoudoro April 29, 2024 21:11

skoudoro reviewed Apr 30, 2024

View reviewed changes

jhlegarreta force-pushed the TransitionToRuffForFormatting branch from ea4ccc0 to 20e1a17 Compare April 30, 2024 22:17

jhlegarreta force-pushed the TransitionToRuffForFormatting branch from 20e1a17 to 40cfa2f Compare April 30, 2024 22:21

jhlegarreta force-pushed the TransitionToRuffForFormatting branch 10 times, most recently from c4349ff to 1320447 Compare May 4, 2024 00:44

jhlegarreta closed this May 4, 2024

jhlegarreta reopened this May 4, 2024

jhlegarreta force-pushed the TransitionToRuffForFormatting branch 11 times, most recently from 88134f5 to 97c9096 Compare May 4, 2024 23:08

jhlegarreta force-pushed the TransitionToRuffForFormatting branch from 97c9096 to 65a12d6 Compare May 4, 2024 23:11

skoudoro approved these changes May 5, 2024

View reviewed changes

skoudoro merged commit ec765e8 into fury-gl:master May 5, 2024
27 of 31 checks passed

skoudoro added the type:Maintenance label May 5, 2024

jhlegarreta deleted the TransitionToRuffForFormatting branch May 5, 2024 14:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

STYLE: Format code using `ruff` #881

STYLE: Format code using `ruff` #881

jhlegarreta commented Apr 27, 2024 •

edited

Loading

jhlegarreta commented Apr 27, 2024

codecov bot commented Apr 27, 2024 •

edited

Loading

skoudoro left a comment

skoudoro left a comment

jhlegarreta commented Apr 30, 2024

skoudoro commented Apr 30, 2024

skoudoro commented Apr 30, 2024

jhlegarreta commented Apr 30, 2024

skoudoro commented Apr 30, 2024

WassCodeur commented May 1, 2024

jhlegarreta commented May 4, 2024

skoudoro commented May 4, 2024

jhlegarreta commented May 4, 2024 •

edited

Loading

jhlegarreta commented May 5, 2024

skoudoro left a comment

STYLE: Format code using ruff #881

STYLE: Format code using ruff #881

Conversation

jhlegarreta commented Apr 27, 2024 • edited Loading

jhlegarreta commented Apr 27, 2024

codecov bot commented Apr 27, 2024 • edited Loading

Codecov Report

skoudoro left a comment

Choose a reason for hiding this comment

skoudoro left a comment

Choose a reason for hiding this comment

jhlegarreta commented Apr 30, 2024

skoudoro commented Apr 30, 2024

skoudoro commented Apr 30, 2024

jhlegarreta commented Apr 30, 2024

skoudoro commented Apr 30, 2024

WassCodeur commented May 1, 2024

jhlegarreta commented May 4, 2024

skoudoro commented May 4, 2024

jhlegarreta commented May 4, 2024 • edited Loading

jhlegarreta commented May 5, 2024

skoudoro left a comment

Choose a reason for hiding this comment

STYLE: Format code using `ruff` #881

STYLE: Format code using `ruff` #881

jhlegarreta commented Apr 27, 2024 •

edited

Loading

codecov bot commented Apr 27, 2024 •

edited

Loading

jhlegarreta commented May 4, 2024 •

edited

Loading